Added Push Camera by dgomes · Pull Request #15151 · home-assistant/core

dgomes · 2018-06-25T23:21:42Z

Description:

Push Camera is a meta-platform that can receive images through the http API.

A use case, is the integration with motioneye. Motioneye is usually configured to save/record files ONLY when motion is detected. It provides a hook to run a command whenever an image is saved, which can be used together with cURL to send the motion detected images to Push Camera, as showed in this example:

curl -X POST -F "image=@%f" http://hassio.test:8123/api/camera_push/camera.my_push_camera

Optionally the Push Camera is able to buffer a given number of images, creating an animation of the detected motion.

Images are cleared on new events, and events are separated by a soft (configurable) timeout.

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5606

Example entry for `configuration.yaml` (if applicable):

camera:
  - platform: push
    buffer: 3
    timeout: 5

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Tests have been added to verify that the new code works.

awarecan · 2018-06-26T02:30:28Z

I have been told not cache image in camera,

Maybe better use await request.multipart() if possible, or at least support that.

Better make 'image' in data['image'].filename configurable.

frenck · 2018-06-26T03:08:13Z

Little notice: Seems like the docs are missing.

MartinHjelmare · 2018-06-26T08:04:45Z

+    async def update_image(self, image, filename):
+        """Update the camera image."""
+        if self._state == STATE_IDLE:
+            self._last_trip = dt_util.utcnow()


This seems to be the same as last_changed state attribute. If that's true, why do we need this?

It is not, this will refer to the moment we start recording, last_changed will be updated during recording. The duration of the recording will be last_changed - last_trip

If state is the same, ie recording, last_changed will not be updated after a state update.

last_trip is only set on the transition STATE_IDLE -> STATE_RECORDING
last_changed is set on both transitions

both attributes will be equal only during the STATE_RECORDING state.

Ok, thanks for clarifying.

MartinHjelmare · 2018-06-26T08:05:48Z

+        self._expired = async_track_point_in_utc_time(
+            self.hass, reset_state, dt_util.utcnow() + self._timeout)
+
+        self.schedule_update_ha_state()


Use the async version here since we're in a coroutine.

MartinHjelmare · 2018-06-26T08:09:25Z

+
+        self.schedule_update_ha_state()
+
+    def camera_image(self):


This isn't needed since we're overwriting the async version of this method below.

MartinHjelmare · 2018-06-26T08:17:45Z

+        try:
+            data = await request.post()
+            _LOGGER.debug("Received Camera push: %s", data['image'])
+            await _camera.update_image(data['image'].file.read(),


Is this api to POST an image standardized or is it specific for this implementation?

It is not standardised, I will make it configurable as suggested by @awarecan

MartinHjelmare · 2018-06-26T08:23:13Z

+    @property
+    def motion_detection_enabled(self):
+        """Camera Motion Detection Status."""
+        return self._motion_status


If this is static, we can just return False here and don't need the instance attribute.

MartinHjelmare · 2018-06-26T08:24:03Z

+        self._filename = None
+        self._expired = None
+        self._state = STATE_IDLE
+        self._timeout = datetime.timedelta(seconds=timeout)


Use the timedelta config validation helper in the schema instead.

MartinHjelmare · 2018-06-26T08:25:04Z

+async def async_setup_platform(hass, config, async_add_devices,
+                               discovery_info=None):
+    """Set up the Push Camera platform."""
+    cameras = [PushCamera(config.get(CONF_NAME),


dict.get isn't needed since there are defaults in place via the config schema.

dgomes · 2018-06-26T09:42:52Z

@awarecan tks for your comment, per default there is no cache only the current image. But for the described use case (motioneye) makes much sense to cache the event. So lets see what others think about it.

Good point on making image configurable, I will change that later tonight.

houndci-bot · 2018-06-26T22:44:56Z

+
+from homeassistant.components.camera import Camera, PLATFORM_SCHEMA,\
+    STATE_IDLE, STATE_RECORDING
+from homeassistant.util.async_ import run_coroutine_threadsafe


'homeassistant.util.async_.run_coroutine_threadsafe' imported but unused

houndci-bot · 2018-06-26T23:35:44Z

+    resp = await client.post('/api/camera_push/camera.wrong', data=files)
+    assert resp.status == 400
+
+@mock.patch("PIL.Image.new")


expected 2 blank lines, found 1

houndci-bot · 2018-06-26T23:35:44Z

+from homeassistant.util import dt as dt_util
+from tests.components.auth import async_setup_auth
+
+@mock.patch("PIL.Image.new")


expected 2 blank lines, found 1

houndci-bot · 2018-06-26T23:35:44Z

+            return self.json_message('Invalid POST', HTTP_BAD_REQUEST)
+        except KeyError as key_error:
+            _LOGGER.error('In your POST message %s', key_error)
+            return self.json_message('Parameter {} missing'.format(self._image), HTTP_BAD_REQUEST)


line too long (98 > 79 characters)

houndci-bot · 2018-06-26T23:37:11Z

+            return self.json_message('Invalid POST', HTTP_BAD_REQUEST)
+        except KeyError as key_error:
+            _LOGGER.error('In your POST message %s', key_error)
+            return self.json_message('Parameter {} missing'.format(self._image),


line too long (80 > 79 characters)

pvizeli

Looks good. But we should work like the other cameras and if there is no picutre, we return none.

dgomes · 2018-06-27T10:48:00Z

Returning none will impact the frontend, if the picture takes to long to appear the browser shows a missing image and not the proposed blank (more user friendly)

Either this logic (create a blank image) moves to the parent class, or the frontend should better handle the None image. Opinions ?

balloob · 2018-06-27T14:00:10Z

Frontend should handle it better, period. No error handling in platforms for things related to the frontend.

balloob · 2018-06-27T14:02:19Z

+
+        self._state = STATE_RECORDING
+        self._filename = filename
+        self.queue.append(image)


I feel like this platform is getting over complicated. It should just show the last pushed image.

We should also store that image on disk and not in memory.

With que, you could see a stream if you push multible images into the entity. Maybe default 1 and you can overwrite this with a option?

This is abusing a feature of our camera integration. The camera get_image function should show a realtime stream of what a camera is showing.

We don't support replays in our camera integration right now. So we shouldn't hack it in.

I feel strongly against writing to disk. Most people run HA on an SD card, it's really bad to write continuously to the SD.

I share @pvizeli comment (that was my intention in the implementation, therefore the current default size of deque = 1).

Let me remind you that the most common use case for this camera is not realtime, and most of the time the user will be faced with the last image (very poor context on what happened).

It could be resolved by allow customized save image path, then point it to some tmpfs.

Let me remind you that the most common use case for this camera is not realtime

I don't like this assumption. If you think you need a well-tuned components only for your use case, probably should not use a such generic name.

@awarecan sure it can (thats how I did before developing this code)

But that's not easy for the average user, and would require hassio/hassos to be aware of that need.

We are not discussing full length video, we are discussing 3 or 5 images (it doesn't take that much memory)

@awarecan I first had this component living in my custom_components repo (name is http_post). But realised people were using it for the described use case.

There is nothing specific in the component related to motionEye, so I did not call it camera.motionEye and even made some changes to make it even more general.

A Raspberry Pi has 1GB of RAM. If 10 images are kept around, 1MB each, we'll be using 1% of memory for this feature.

@balloob what is now the plan, remove or hold?

dgomes · 2018-06-27T20:16:19Z

Camera Push will now return image None before first image is received.

pvizeli · 2018-07-01T20:19:43Z

+        if self._expired:
+            self._expired()
+
+        self._expired = async_track_point_in_utc_time(


rename this like _expired_listener so that you know what it is

pvizeli · 2018-07-01T20:20:13Z

+            """Set state to idle after no new images for a period of time."""
+            self._state = STATE_IDLE
+            self.async_schedule_update_ha_state()
+            self._expired = None


first set all variable and after that call the function

pvizeli · 2018-07-01T20:22:39Z

+            self._last_trip = dt_util.utcnow()
+            self.queue.clear()
+
+        self._filename = filename


do we really need this? That make a state change any new picture is pushed

That is actually a feature :)

That state change will help automations that require a trigger (e.g. do some image_processing, send a notification).

Yeah, that will be trigger if the state of camera going to recording, why you need after that a state change?

In the use case of motioneye, an event will consist of 3 images (default)

I expect to be notified of the 3 for the aforementioned automations.

I have a idea. Add the options force_update (default false) and use this on entity as attribute force_update.
Other platform make the same. That allow user to force state change on if a update is exists. I think it is a bit wired to store a filename into attribute to have the same effect. What do you think?

That is possible yes, I can work that out tonight.

But I also believe the filename can be a useful attribute by itself

For what? Can you give a example why to you need this attribute?

Filename is a unique key, and can be used to trace events.

Personal example:
MotionEye sends HA an image, HA sends a notification with the image. If I want to go back to motionEye image storage, I need a key to look for. Sending the filename with the notification lets me go back to motionEye storage and look for the image and review the security event (way past the time HA has cleared the image from the queue)

Ok. Remove the force_update if you want staying with that attribute and we can merge it.

pvizeli · 2018-07-03T10:05:55Z

+
+PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
+    vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
+    vol.Optional(CONF_BUFFER_SIZE, default=1): cv.positive_int,


maybe queue_size ? I'm not sure if buffer_size to odd

It's a buffer... that happens to be implemented with a queue.

I don't think it makes sense to rename it.

This reverts commit e203785.

mezz64 · 2018-07-06T19:40:53Z

I know this has already been merged, but it presently doesn't seem to actually allow pushing images to multiple defined entities, only the first one configured. All others throw a push camera not found error. The camera card and associated entities are properly created for all defined cameras you just can't post data to any one but the first.

dgomes · 2018-07-06T21:44:21Z

@mezz64 Tks for the bug report, but this is not the proper place :)

I'm already issuing a new PR with a fix!

Added push camera

1841054

homeassistant added cla-signed new-platform platform: camera.push labels Jun 25, 2018

ghost assigned dgomes Jun 25, 2018

ghost added the in progress label Jun 25, 2018

add camera.push

7a7b1a8

homeassistant added the cla-signed label Jun 25, 2018

dgomes changed the title ~~Added Push Camera~~ WIP: Added Push Camera Jun 26, 2018

MartinHjelmare reviewed Jun 26, 2018

View reviewed changes

frenck added the docs-missing label Jun 26, 2018

Address comments and add tests

0cbf418

homeassistant added the cla-signed label Jun 26, 2018

houndci-bot reviewed Jun 26, 2018

View reviewed changes

dgomes added 2 commits June 26, 2018 23:46

auff auff

7143c95

trip time made no sense

9294ceb

dgomes mentioned this pull request Jun 26, 2018

Add Camera Push home-assistant/home-assistant.io#5606

Merged

2 tasks

dgomes changed the title ~~WIP: Added Push Camera~~ Added Push Camera Jun 26, 2018

dgomes added 2 commits June 27, 2018 00:27

travis lint

704ad99

Mock dependency

c914c08

homeassistant added the cla-signed label Jun 26, 2018

dgomes removed the docs-missing label Jun 26, 2018

houndci-bot reviewed Jun 26, 2018

View reviewed changes

hound

50087e1

homeassistant added the cla-signed label Jun 26, 2018

long line

12c8a4d

houndci-bot reviewed Jun 26, 2018

View reviewed changes

pvizeli requested changes Jun 27, 2018

View reviewed changes

balloob reviewed Jun 27, 2018

View reviewed changes

dgomes added 4 commits June 27, 2018 20:21

remove blank image

1196a0a

no more need to mock dependency

7f7c326

remove import

2247d19

cleanup

a8eaece

homeassistant added the cla-signed label Jun 27, 2018

no longer needed

cb6710d

unused constant

cfb17a0

pvizeli reviewed Jul 1, 2018

View reviewed changes

pvizeli self-assigned this Jul 1, 2018

address @pvizeli review

b79a133

homeassistant added the cla-signed label Jul 1, 2018

add force_update

e203785

pvizeli reviewed Jul 3, 2018

View reviewed changes

dgomes added 2 commits July 3, 2018 23:11

Revert "add force_update"

469219c

This reverts commit e203785.

rename parameter

53063ab

pvizeli approved these changes Jul 4, 2018

View reviewed changes

pvizeli merged commit 5f7ac09 into home-assistant:dev Jul 4, 2018

ghost removed the Ready for review label Jul 4, 2018

dgomes mentioned this pull request Jul 6, 2018

fix camera.push API overwrite #15334

Merged

8 tasks

home-assistant locked as resolved and limited conversation to collaborators Jul 7, 2018

ghost added integration: push and removed platform: camera.push labels Mar 21, 2019

dgomes deleted the push.camera branch March 11, 2022 22:28

Uh oh!

Conversation

dgomes commented Jun 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

awarecan commented Jun 26, 2018

Uh oh!

frenck commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgomes Jun 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgomes commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli left a comment

Choose a reason for hiding this comment

Uh oh!

dgomes commented Jun 27, 2018

Uh oh!

balloob commented Jun 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgomes Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgomes commented Jun 27, 2018

dgomes commented Jun 25, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

dgomes Jun 26, 2018 •

edited

Loading

dgomes Jun 27, 2018 •

edited

Loading

pvizeli Jul 2, 2018 •

edited

Loading